-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Swap from withPolicy to withPolicyConnections for Export Quickbooks pages #40713
Swap from withPolicy to withPolicyConnections for Export Quickbooks pages #40713
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We defined defaultCards. Are those drafts only (testing only) or meant to stay?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah - i think creditCards should be used in the bottom menu item(currently not available) and here should be just used defaults based on the high level doc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If creditCards exist on quickbooksOnline.data
then I would assume we want to use those
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we're discussing this one here and if so, we'll handle it in a follow-up.
src/pages/workspace/accounting/qbo/export/QuickbooksExportInvoiceAccountSelectPage.tsx
Outdated
Show resolved
Hide resolved
src/pages/workspace/accounting/qbo/export/QuickbooksOutOfPocketExpenseAccountSelectPage.tsx
Show resolved
Hide resolved
src/pages/workspace/accounting/qbo/export/QuickbooksPreferredExporterConfigurationPage.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If creditCards exist on quickbooksOnline.data
then I would assume we want to use those
src/pages/workspace/accounting/qbo/export/QuickbooksOutOfPocketExpenseAccountSelectPage.tsx
Outdated
Show resolved
Hide resolved
src/pages/workspace/accounting/qbo/export/QuickbooksOutOfPocketExpenseAccountSelectPage.tsx
Outdated
Show resolved
Hide resolved
src/pages/workspace/accounting/qbo/export/QuickbooksOutOfPocketExpenseAccountSelectPage.tsx
Outdated
Show resolved
Hide resolved
@s77rt this from high level doc: |
@s77rt also flow will changed a bit in next follow up. we will have additional screen to pick creditCards from api (screen at right): |
Reviewer Checklist
Screenshots/VideosAndroid: NativeAndroid: mWeb ChromeiOS: mWeb SafariMacOS: Desktop |
Do we need NO QA in the title? |
@hayata-suenaga can probably answer that, also Hayata are you going to do the secondary review on this? |
sorry I forgot to answer to this question. Yes, we need QA. I added the detailed QA steps in the OP of this issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟢 to me. Let's merge and left QA test the flow
Last second 😂 |
🚀 Deployed to staging by https://github.com/hayata-suenaga in version: 1.4.65-0 🚀
|
🚀 Deployed to production by https://github.com/mountiny in version: 1.4.65-5 🚀
|
Details
Follow up integration withPolicyConnection inside Export pages
Fixed Issues
$ #39108
PROPOSAL:
Tests / QA Steps
Applause team: Please don't add deploy blocker to the issue that you create if you find any issues with the following QA steps. This is behind beta, and we don't have to block deploy on this. Please just create GH issues without the deploy blocker label
Accounts Payable
,Other Current Assets
, andOther Current Liabilities
Bank
.Accounts Payable
.Accounts Receivable
.Credit Card
.Bank
.Accounts Payable
.Offline tests
N/A
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop